-
Notifications
You must be signed in to change notification settings - Fork 736
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Cannot set mail.force_extra_parameters as PHP_INI_PERDIR #1081
base: master
Are you sure you want to change the base?
Conversation
reference/mail/ini.xml
Outdated
@@ -31,7 +31,7 @@ | |||
<row> | |||
<entry><link linkend="ini.mail.force_extra_parameters">mail.force_extra_parameters</link></entry> | |||
<entry>NULL</entry> | |||
<entry>PHP_INI_SYSTEM|PHP_INI_PERDIR</entry> | |||
<entry>&php.ini; only</entry> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a tricky one, since the setting can be set in httpd.conf as php_value
what is usually only allowed for PHP_INI_PERDIR
and PHP_INI_ALL
. Maybe leave this line as is:
<entry>&php.ini; only</entry> | |
<entry>PHP_INI_SYSTEM|PHP_INI_PERDIR</entry> |
And change the following line:
<entry>&php.ini; only</entry> | |
<entry>Cannot be changed in <filename>.htaccess</filename> and <filename>.user.ini</filename></entry> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cmb69 If httpd.conf is also allowed that means that https://github.com/php/doc-en/blob/3cdd39bb9505e6735d802da83a04870cfa8f2311/appendices/ini.list.xml is also incorrect.
And is PHP_INI_PERDIR minus .user.ini and .htaccess not effectively the same as PHP_INI_SYSTEM? (according to the options on https://www.php.net/manual/en/configuration.changes.modes.php)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PHP_INI_SYSTEM
only allows to set the value via php_admin_value
, but mail.force_extra_parameters
can be set via php_value
in .httpconf. Frankly, I wonder why that setting wasn't simply changed to PHP_INI_SYSTEM
, but we have to document the status quo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cmb69 Are you absolutely sure mail.force_extra_parameters
can be set through the httpd config, and is not being reverted by OnChangeMailForceExtra
specified in https://github.com/php/php-src/blob/PHP-7.1.1/main/main.c#L601 ?
Whatever the documented outcome should be, it must be applied to both files (ini.list.xml and ini.xml)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OnChangeMailForceExtra
is only there to prevent modifications in .htaccess and .user.ini. Setting mail.force_extra_parameters
has effect in .htconf via php_value
(changes the local value; master value remains, as usual).
Whatever the documented outcome should be, it must be applied to both files (ini.list.xml and ini.xml)
Indeed, although we should try to get rid of the duplication. If we won't care about the alphabetic order of the directives, the solution would be trivial. Otherwise, this is more ugly/brittle.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understand correctly this setting can be set in httpd-config through only with php_value
and not with php_admin_value
? Or can it be set either way?
Both documentation pages only allow for a very short description under column "Changeable". Therefor I would vote to document this in the most concise and clear way possible.
Some suggestions:
PHP_INI_SYSTEM
(if detail can be ommitted)PHP_INI_SYSTEM with php_value
php.ini, or httpd.conf with php_value
Technical nuances could also be added under https://www.php.net/manual/en/mail.configuration.php#ini.mail.force_extra_parameters
Pinging author: @marcovtwout |
Co-authored-by: Christoph M. Becker <[email protected]>
Hmm, as it is now, the PR would not change anything. Are you still working on that @marcovtwout? :) |
@cmb69 There has been no activity and it's still a zero-line PR. Does it make sense to close it? |
@cmb69 I think we were almost there, we just have to pick up the discussion where we left off in the conversation above #1081 (comment) If you could answer my remaining question there, I'll update the PR with one of the suggested solutions. |
Make it |
Closing and reopening to see what the pipeline is complaining about. Logs were too old to view. |
cannot set mail.force_extra_parameters as PHP_INI_PERDIR
Synchronise change from reference/mail/ini.xml
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has no description of the nuance Christopher has said.
This should be documented in the mail/ini.xml
file, under the relevant INI description section.
@Girgias You're right it doesn't, because I'm not entire sure what to put there exactly. My previous question is still awaiting reply:
|
Documentation does not match: https://github.com/php/doc-en/blob/3cdd39bb9505e6735d802da83a04870cfa8f2311/appendices/ini.list.xml
This param cannot be set through PHP_INI_PERDIR because of https://bugs.php.net/bug.php?id=71901